Conversation
ba40c18 to
2a29077
Compare
|
oopsie.. |
b5836c0 to
f8b1807
Compare
There was a problem hiding this comment.
Please complete the new port checklist:
- Changes comply with the maintainer guide.
- The name of the port matches an existing name for this component on https://repology.org/ if possible, and/or is strongly associated with that component on search engines.
- Optional dependencies are resolved in exactly one way. For example, if the component is built with CMake, all
find_packagecalls are REQUIRED, are satisfied byvcpkg.json's declared dependencies, or disabled with CMAKE_DISABLE_FIND_PACKAGE_Xxx. - The versioning scheme in
vcpkg.jsonmatches what upstream says. - The license declaration in
vcpkg.jsonmatches what upstream says. - The installed as the "copyright" file matches what upstream says.
- The source code of the component installed comes from an authoritative source.
- The generated "usage text" is accurate. See adding-usage for context.
- The version database is fixed by rerunning
./vcpkg x-add-version --alland committing the result. - Only one version is in the new port's versions file.
- Only one version is added to each modified port's versions file.
| "asm" | ||
| ], | ||
| "features": { | ||
| "asm": { |
There was a problem hiding this comment.
It seems like this shouldn't be a feature; if anything it would be something someone may want to force off, and that's not what features are for. (In particular, in vcpkg's model it is always assumed that turning on a feature is 'safe')
There was a problem hiding this comment.
So, according to vcpkg's model, this should not be in the port at all? And it should decide whether to build assemblies or not on its own? If feature in vcpkg suppose to enable some specific optional modules let's say, that makes sense.
Also, in this case the feature should actually be safe to use as it's the core funtionality of the library.
|
Can you describe how this library avoids taking symbols defacto owned by OpenSSL? Things like https://github.com/rinrab/xdigest/blob/0a0c673c5d982733bb7198bb89d1c65b3ab254d6/xdigest/include/internal/common.h#L48 do not fill one with confidence that this can be installed without introducing multiple definition errors if OpenSSL is also installed. |
All the public symbols SHOULD be renamed. This can be checked with |
This library was developed by me in recent days and I wish it was added to the vcpkg repository. For the rest title and portfile are self explanatory.
Project's homepage can be found here on github: https://github.com/rinrab/xdigest
find_packagecalls are REQUIRED, are satisfied byvcpkg.json's declared dependencies, or disabled with CMAKE_DISABLE_FIND_PACKAGE_Xxx.vcpkg.jsonmatches what upstream says.vcpkg.jsonmatches what upstream says../vcpkg x-add-version --alland committing the result.